Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Application::run returns on native platforms #1112

Merged
merged 3 commits into from
Nov 29, 2021

Conversation

AndiHofi
Copy link

@AndiHofi AndiHofi commented Nov 7, 2021

Added an on_exit() -> Option<Box<dyn FnOnce>> function to the iced::Application trait that provides that callback.

When the returned closure does not actually exit the application then iced::Application::run() can return as well, allowing the program to continue when the main window is closed.

For iced_web it could make sense to invoke the on_exit() closure when unloading the page, did not try to implement that though.

The main use-case for this is to allow to change the exit code of the application.

Copy link
Member

@hecrj hecrj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I'm missing something, but if we are leveraging run_return and allowing Application::run to return control, then the on_exit method seems redundant.

In other words, users can just write:

let _ = Application::run()?;

on_exit();

@AndiHofi
Copy link
Author

AndiHofi commented Nov 8, 2021

yeah - I somehow overlooked that. I did not want to change the control flow of Application::run() it currently only returns when initializing the application fails.

I realized that the run() method may return normally when running through the code for the pull-request.

Letting it return in all cases would be simpler than any callback - unless returning from Application::run does not work with all backends.

@AndiHofi
Copy link
Author

Removed the on_exit() callback entirely and added an example that Application::run returns. Also that it may be called in a loop.

Added another commit that demonstrates the same when calling iced_winit::run or iced_glutin::run

Copy link
Member

@hecrj hecrj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we keep the changes to the the bare minimum (i.e. just the run_return)?

We can think about additional examples later. If we start adding examples for every change, they will be very hard to navigate 😅

@AndiHofi
Copy link
Author

sure, I get rid of the examples...

and rename the pull request. There is no more on_exit callback

@AndiHofi AndiHofi changed the title Add support for an optional on_exit callback Application::run returns on native platforms Nov 21, 2021
@hecrj hecrj added this to the 0.4.0 milestone Nov 25, 2021
@hecrj hecrj added the feature New feature or request label Nov 25, 2021
Copy link
Member

@hecrj hecrj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

I made some minor changes, but it should be ready to go! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants